Use one grid cell per model input, and have the method name span multiple rows#2995
Conversation
| <> | ||
| <DataGridCell> | ||
| <ModelTypeDropdown | ||
| key={index} |
There was a problem hiding this comment.
I was a little concerned about removing these keys. I've done manual tested and I haven't seen anything not updating correctly. Also, the default behaviour if you don't specify a key is to use the index.
There was a problem hiding this comment.
I think the key can be removed here since this is not a top-level element in the map anymore. However, I think we should still add a key on the <> that is now the top-level literal. The only reason ESLint doesn't give an error is that it doesn't seem to detect it on <>, but changing it to its concrete implementation (using <Fragment> instead of <>) does give the error.
If we don't add the key there, we will get an error in the next major version of eslint-react: https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/jsx-key.md#checkfragmentshorthand-default-false. It's probably best to add it by changing it to <Fragment key={index}>.
There was a problem hiding this comment.
Ah nice. I did try adding a key but it won't even allow it on a <>. Good suggestion to switch to <Fragment> instead.
2 participants
This follows on from #2990.
This addresses the point of using
MultiModelColumnto fit multiple inputs for a certain field but different models into a single grid cell. Instead, we use one grid cell per input, and usegridRow="span N"to make the method name span multiple rows.Here's what it looks like:
I've used a narrow window width so things flow onto multiple lines. The grid lines are visible because I've highlighted that element in the developer tools. Sadly the grid lines don't show where cells span multiple rows/columns, but you can see by the contents that it's working.
Checklist
ready-for-doc-reviewlabel there.